Skip to content

refactor(odoo): Core-first correction — structural facts home in the typed Core, harvest subordinate#530

Merged
AdaWorldAPI merged 2 commits into
mainfrom
claude/odoo-spo-selection-values
Jun 18, 2026
Merged

refactor(odoo): Core-first correction — structural facts home in the typed Core, harvest subordinate#530
AdaWorldAPI merged 2 commits into
mainfrom
claude/odoo-spo-selection-values

Conversation

@AdaWorldAPI

@AdaWorldAPI AdaWorldAPI commented Jun 18, 2026

Copy link
Copy Markdown
Owner

Summary

Repurposed. This started as "add selection_value as spo_enrich.py predicate #5 (wishlist P3)." The operator caught a real architectural inversion before merge, so it is now the Core-first correction: the wishlist's structural asks belong in the typed OdooEntity Core (lance-graph-ontology::odoo_blueprint), not bolted onto the flat SPO ndjson by the AST harvest.

The inversion (source-verified)

target / inverse_name / inherits_from / selection_value are Core facts — relations, composition, value-domain — per the Core-first transcode doctrine. Checking the actual typed Core:

Predicate State in the typed OdooEntity Core Verdict
target / inverse_name OdooField.target already exists harvest was re-deriving a Core fact
inherits_from OdooEntityKind = ORM base class only, no mixin list Core gap filled on the harvest side
selection_value OdooFieldKind::Selection flags it; no values slot Core gap filled on the harvest side

Filling a Core gap on the harvest side is the doctrine's named anti-pattern: "a Core gap → extend the Core deliberately, never hack the adapter."

Why a typed side-table (not a struct field)

OdooField has 3 554 literal sites, OdooEntity 404, no constructor — adding a field to the mega-structs breaks every literal. The doctrine-correct "extend the Core deliberately" for a literal sea that size is a typed side-table: still Core, still authoritative, beside the mega-structs.

What lands

New crates/lance-graph-ontology/src/odoo_blueprint/structural.rs:

Reframing — so a future session does not read the prior work as "add another spo_enrich predicate":

  • spo_enrich.py module-doc gains an ARCHITECTURE NOTE: the structural predicates' authoritative home is the typed Core; this file is the Extracted-leg breadth feeder for the ~322 ObjectTypes the curated Core hasn't reached; the Curated Core wins on convergence in the SpoStore. Behavioural predicates (deep reads_field, emitted_by, validation_kind) ARE genuine harvest and correctly stay.
  • EPIPHANIES.md prepends E-ODOO-CORE-FIRST-STRUCTURAL, which explicitly bounds the framing of the four prior E-ODOO-SPO-* entries (append-only; they're retained, their predicate-bolt-on cadence is no longer the headline). Names virtually_overrides as a ClassView/Core capability — not harvest predicate feat(graph): add SPO triple store with bitmap ANN, TruthGate, semirin… #6.

Two-leg convergence (the standing shape)

Per odoo-extraction-strategies-v1.md: the Curated leg (typed Core: this module + the 66 L-doc entities) is authoritative; the Extracted leg (spo_enrich.py) is breadth for uncurated models, subordinate. Structural facts live in the Core and project out to SPO; the harvest fills in where the Core is silent. This is the intended architecture — the drift was presenting the harvest as the home, not the legs existing.

What is unchanged

The selection_value AST reader in spo_enrich.py stays — it's the Extracted leg's mechanism for breadth — now correctly subordinate. The merged inherits_from/validation_kind/target/deep-reads_field passes (#523/#526/#527) are not reverted; their framing is corrected forward via the EPIPHANIES entry, and their structural outputs are now superseded-as-source by the typed Core where it covers a model.

Tests

  • cargo test -p lance-graph-ontology --lib structural5/5
  • python3 -m unittest tests.test_spo_enrich53/53
  • cargo test -p lance-graph --lib odoo_ontology13/13 (unchanged)

Follow-up (not in this PR)

  • Widen the curated structural.rs registries beyond account.move as L-doc curation reaches more models.
  • virtually_overrides → the typed Core's class-resolution surface (ClassView), not a harvest predicate.

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@AdaWorldAPI, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 55 minutes and 33 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d2b01631-d60f-44de-b891-64d6d0b5b83f

📥 Commits

Reviewing files that changed from the base of the PR and between 402ab09 and 502a255.

📒 Files selected for processing (6)
  • .claude/board/EPIPHANIES.md
  • crates/lance-graph-ontology/src/odoo_blueprint/mod.rs
  • crates/lance-graph-ontology/src/odoo_blueprint/structural.rs
  • crates/lance-graph/src/graph/spo/odoo_ontology.rs
  • tools/odoo-blueprint-extractor/odoo_blueprint_extractor/spo_enrich.py
  • tools/odoo-blueprint-extractor/tests/test_spo_enrich.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0c9cb383fb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +151 to +154
for kw in call.keywords:
if kw.arg == "selection":
sel = kw.value
break

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include selection_add keys in selection_value

When an addon extends an existing Odoo selection field with _inherit and fields.Selection(selection_add=[('x', 'X')]), these lines never inspect the statically known selection_add keyword, so the enrichment emits only the base field's values. That makes the generated selection_value set incomplete, and a downstream ASSERT $value IN [...] built from it would reject records using legal extension-added states.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 21d8ef0. Real finding — addons extend a Selection field's domain via _inherit + selection_add=[...] without redeclaring the base list. Two changes:

  1. _extract_selection_values now reads both the base list (positional / selection=) and selection_add=, unioned base-first with order-preserving dedup.
  2. _scan_file binding merges keys per (model, field) instead of last-write-wins — base and selection_add live in different classes, so the value set accumulates across scans.

Cross-class regression test added: base state in the _name class + selection_add=[('reviewed', ...)] in an _inherit class → accumulated [draft, posted, reviewed]. 57/57 tests.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0c9cb383fb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +151 to +154
for kw in call.keywords:
if kw.arg == "selection":
sel = kw.value
break

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include selection_add keys in selection_value

When an addon extends an existing Odoo selection field with _inherit and fields.Selection(selection_add=[('x', 'X')]), these lines never inspect the statically known selection_add keyword, so the enrichment emits only the base field's values. That makes the generated selection_value set incomplete, and a downstream ASSERT $value IN [...] built from it would reject records using legal extension-added states.

Useful? React with 👍 / 👎.

Adds a fifth enrichment pass to spo_enrich.py: `fields.Selection`
declarations with a statically-resolvable list of 2-tuples emit one
`(odoo:<model>.<field>, selection_value, "<key>")` triple per enum key.

# Shape

`_extract_selection_values(call)` pulls the first element of each 2-tuple
from the Selection list — positional arg 0 OR `selection=` kwarg —
preserving source order, de-duplicating (first occurrence wins).

Skipped (values not statically knowable):
  - `selection='_compute_states'` (compute-method ref, a str)
  - `selection=STATE_CONSTANT` (bare Name)
  - `related=...` / any non-list/tuple selection arg
  - individual entries that aren't 2-tuples

Truth `(0.95, 0.90)` — read straight from the field decorator,
authoritative. Scoped to corpus-declared ObjectTypes (the additive
boundary, same as P1); Selection fields bind to the same `model_names`
as relational fields (`_name`, else `_inherit[0]`, per #525's decision).

# Consumer use

Lets odoo-rs lower a Selection field to
`DEFINE FIELD state … ASSERT $value IN ['draft','posted','cancel']`
— the UPSTREAM_WISHLIST P3 ask. The five-pass enrichment is now:
P1 (target/inverse_name) + P0 (deep reads_field) + P1b (inherits_from)
+ P2 (validation_kind) + P3 (selection_value).

# Corpus regen pending

The shipped corpus does not yet carry selection_value triples —
regenerating requires running the script against a live Odoo source
tree (`/home/user/odoo/addons`), not present on this host. The Rust
loader's predicate-histogram match arm gained `selection_value` so a
future regenerated corpus drops into the harness without code changes.
`parses_all_triples` count assertion stays at 24 579; re-locks the
moment a session with the source re-runs enrichment.

# Files

  spo_enrich.py:
    +`_extract_selection_values` helper, +`SELECTION_VALUE_TRUTH`,
    +`selections` param threaded through `_scan_file` / `build_all_facts`
    / `enrich`, +Selection branch in the field loop, +P3 emission loop,
    +CLI status field.
  test_spo_enrich.py:
    +12 tests (6 extraction edge cases + 3 scan-binding + 3 emission).
  odoo_ontology.rs:
    +doc table row, +histogram match arm for `selection_value`.
  EPIPHANIES.md: prepended E-ODOO-SPO-SELECTION-VALUE.

# Tests

  python3 -m unittest tests.test_spo_enrich : 53/53 OK (was 41)
  cargo test -p lance-graph --lib odoo_ontology : 13/13 OK
@AdaWorldAPI AdaWorldAPI force-pushed the claude/odoo-spo-selection-values branch from 0c9cb38 to 6ded9e0 Compare June 18, 2026 06:57
…d Core, harvest is subordinate

Repurposes the P3 selection_value work after the operator caught a real
architectural inversion: the wishlist's structural asks (target /
inverse_name / inherits_from / selection_value) are CORE facts, not
behavioural harvest, and belong in the typed `OdooEntity` Core
(`lance-graph-ontology::odoo_blueprint`) — not bolted onto the flat SPO
ndjson by `spo_enrich.py`.

Source-verified: `OdooField.target` ALREADY exists in the Core, so the
`target` harvest pass was re-deriving a fact the Core already held;
`inherits_from` + `selection_value` were genuine Core gaps filled on the
harvest side — the "Core gap → extend the Core deliberately, never hack
the adapter" anti-pattern.

# Why a side-table, not a struct field

`OdooField` has 3 554 literal sites, `OdooEntity` 404, no constructor.
Adding a field to the mega-structs breaks every literal. The
doctrine-correct "extend the Core deliberately" for a literal sea that
size is a TYPED SIDE-TABLE — still Core, still authoritative, beside the
mega-structs instead of inside them.

# What lands

New `odoo_blueprint/structural.rs`:
  - `OdooInherits { model, bases }` — the _inherit/_inherits mixin chain
    (OdooEntityKind records the ORM base class, not the mixin list)
  - `OdooFieldSelection { model, field, values }` — Selection value domain
    (OdooFieldKind::Selection flags it; OdooField never stored the values)
  - Curated `account.move` consts, GROUNDED: INHERITS matches the #527
    corpus (mail_activity_mixin + sequence_mixin); FIELD_SELECTIONS uses
    canonical standard Odoo state/move_type value sets.
  - `project_inherits_from` / `project_selection_value` — Core → SPO
    projection (direction of truth: Core out to SPO, never the reverse).
  - 5 tests incl. a consistency check against the #527 corpus.

# Reframing (anti-self-fulfilling-drift)

So a future session does NOT read the prior work as "add another
spo_enrich predicate":

  - `spo_enrich.py` module-doc gains an ARCHITECTURE NOTE: the structural
    predicates' authoritative home is the typed Core; this file is the
    Extracted-leg BREADTH feeder for the ~322 ObjectTypes the curated
    Core hasn't reached; Curated Core wins on convergence. Behavioural
    predicates (deep reads_field, emitted_by, validation_kind) ARE
    genuine harvest and correctly stay.
  - `EPIPHANIES.md` prepends E-ODOO-CORE-FIRST-STRUCTURAL, which
    explicitly BOUNDS THE FRAMING of the four prior E-ODOO-SPO-* entries
    so the predicate-bolt-on cadence isn't read as the intended
    architecture. Names `virtually_overrides` as a ClassView/Core
    capability — NOT harvest predicate #6.

# Not changed

The selection_value AST reader in spo_enrich.py stays (it's the Extracted
leg's mechanism for breadth) — now correctly subordinate to the Core.
Behavioural harvest is untouched.

# Tests

  cargo test -p lance-graph-ontology --lib structural : 5/5
  python3 -m unittest tests.test_spo_enrich            : 53/53
  cargo test -p lance-graph --lib odoo_ontology        : 13/13 (unchanged)
@AdaWorldAPI AdaWorldAPI changed the title feat(odoo-spo): selection_value (wishlist P3) refactor(odoo): Core-first correction — structural facts home in the typed Core, harvest subordinate Jun 18, 2026
@AdaWorldAPI AdaWorldAPI merged commit c029f79 into main Jun 18, 2026
6 checks passed
AdaWorldAPI pushed a commit that referenced this pull request Jun 18, 2026
Odoo addons extend an existing Selection field's domain via
`_inherit` + `fields.Selection(selection_add=[('reviewed','Reviewed')])`
WITHOUT redeclaring the base list. `_extract_selection_values` only read
the base `selection=` arg, so the emitted `selection_value` set was
incomplete and a downstream `ASSERT $value IN [...]` would reject records
in legal extension-added states.

Two fixes:

1. `_extract_selection_values` now reads BOTH the base list (positional
   arg 0 or `selection=`) AND `selection_add=`, unioning base-first with
   order-preserving dedup (factored into `_selection_tuple_keys`).

2. `_scan_file` binding MERGES selection keys per (model, field) instead
   of last-write-wins — the base declaration and the `selection_add`
   extension live in DIFFERENT classes, so the field's value set must
   accumulate across scans, not overwrite.

Tests (53 -> 57):
  - selection_add kwarg alone -> added keys
  - base + selection_add -> union, base first
  - selection_add dedups against base
  - cross-class merge: base in _name class + selection_add in _inherit
    class -> accumulated [draft, posted, reviewed]

  python3 -m unittest tests.test_spo_enrich : 57/57 OK
AdaWorldAPI added a commit that referenced this pull request Jun 18, 2026
fix(odoo-spo): read selection_add + merge across classes (codex #530 follow-up)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants